Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add attribute noting details of scrubbed values #278

Merged
merged 12 commits into from
Jul 5, 2024
Merged

Add attribute noting details of scrubbed values #278

merged 12 commits into from
Jul 5, 2024

Conversation

alexmojaki
Copy link
Contributor

@alexmojaki alexmojaki commented Jun 25, 2024 •
edited
Loading

For #248

The idea is to use this to generate code for the user to whitelist values from scrubbing. Here's some example code which I imagine being ported to the frontend:

attributes = {
    'logfire.scrubbed': [
        {'path': ['attributes', 'usage'], 'matched_substring': 'token'},
        {'path': ['attributes', 'prompt'], 'matched_substring': 'secret'},
    ],
}

print(f"""
import logfire

def scrubbing_callback(match: logfire.ScrubMatch):
{"\n".join(f"""\
    if match.path == {tuple(match['path'])} and match.pattern_match.group(0) == {repr(match['matched_substring'])}:
        return match.value
"""
           for match in attributes['logfire.scrubbed'])}

logfire.configure(scrubbing=logfire.ScrubbingOptions(callback=scrubbing_callback))
""")

This generates:

import logfire

def scrubbing_callback(match: logfire.ScrubMatch):
    if match.path == ('attributes', 'usage') and match.pattern_match.group(0) == 'token':
        return match.value

    if match.path == ('attributes', 'prompt') and match.pattern_match.group(0) == 'secret':
        return match.value


logfire.configure(scrubbing=logfire.ScrubbingOptions(callback=scrubbing_callback))

@cloudflare-workers-and-pages Cloudflare Workers and Pages
Copy link

cloudflare-workers-and-pages bot commented Jun 25, 2024 •
edited
Loading

Deploying logfire-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: e545df4
Status: ✅  Deploy successful!
Preview URL: https://372a62bb.logfire-docs.pages.dev
Branch Preview URL: https://alex-scrubbed-note.logfire-docs.pages.dev

View logs

@codecov Codecov
Copy link

codecov bot commented Jun 25, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@alexmojaki alexmojaki marked this pull request as ready for review June 27, 2024 19:17
samuelcolvin
samuelcolvin approved these changes Jul 4, 2024
View reviewed changes
Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code here looks good.

What do we need to change in the backend?

The PR body seems kind of confusing, and/or related to something slightly different. Am I being dumb or should or be updated.

@alexmojaki
Copy link
Contributor Author

The PR body seems kind of confusing, and/or related to something slightly different. Am I being dumb or should or be updated.

It's related. The first bit of code is a sample of attributes that this PR adds to a span:

attributes = {
    'logfire.scrubbed': [
        {'path': ['attributes', 'usage'], 'matched_substring': 'token'},
        {'path': ['attributes', 'prompt'], 'matched_substring': 'secret'},
    ],
}

The rest of that code block is an example of how the attributes can be used to generate sample Python code which is displayed to the user for them to copy and paste to whitelist values from being scrubbed. That's the ultimate goal here.

That code generation logic could also be baked into the SDK, but I think it makes more sense to do that part in the frontend.

The last code block is the output of the first, i.e. the sample code which would be generated and shown to the user.

The backend doesn't really need anything. But since the new attribute is stored as a string and it's not mentioned in the JSON schema, I think the backend should explicitly check for this attribute and parse the JSON just in case someone wants to query it or something.

@alexmojaki alexmojaki force-pushed the alex/scrubbed-note branch from a8e7d5c to e545df4 Compare July 5, 2024 11:50
@alexmojaki alexmojaki enabled auto-merge (squash) July 5, 2024 11:51
@alexmojaki alexmojaki merged commit b04c3a3 into main Jul 5, 2024
10 checks passed
@alexmojaki alexmojaki deleted the alex/scrubbed-note branch July 5, 2024 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samuelcolvin samuelcolvin samuelcolvin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@alexmojaki @samuelcolvin